[RNE Rewrite] fix(fetcher): make iOS resume actually append, and never cache a short file - #1365
Merged
Merged
Conversation
Member
|
Two additional things I noticed when working with LLMs, that maybe you could take a look at:
|
Member
Author
I had that too, but I thought that was because of my bad internet connection. 😅 |
msluszniak
force-pushed
the
@ms/fetcher-verify-download-size
branch
2 times, most recently
from
August 17, 2026 12:09
483e6d7 to
93d265d
Compare
msluszniak
force-pushed
the
@ms/fetcher-verify-download-size
branch
from
August 17, 2026 12:24
93d265d to
491f5d1
Compare
Three defects in the iOS download path, found while testing the CV demo
app on device. Two cached models were sitting at roughly 47% of their real
size under their final names, and ExecuTorch failed on them with
InvalidProgram.
1. Resume silently copied nothing.
`RNBlobUtil.fs.appendFile(path, src, 'uri')` takes a filesystem PATH, but
we passed `file://${target}`. blob-util strips only a `bundle-assets://`
prefix and hands the rest to `NSInputStream initWithFileAtPath:`, so the
stream failed to open, the copy loop never ran, and the call RESOLVED
having written nothing. The un-grown `.partial` was then renamed to its
final cache name, which is where the truncated models came from. The 47%
is simply how far each download had got before the model was switched.
2. A short file could be cached at all.
Nothing compared the assembled length against the file's real length
before the rename; iOS checked only the HTTP status and Android only
rejected an empty file. Since the cache-hit check is existence-only, a
truncated file was then served on every later run with no recovery short
of `forceDownload`. The expected length now comes from a
redirect-following HEAD that `download()` already performs for progress
weighting, so it is threaded through rather than re-requested, resume
assembly is decided by byte counts, and the merged size is confirmed
after appending.
3. Interrupting a resumed download threw away everything it had fetched.
A resumed range lands in a `.chunk` file that was only folded into the
partial on success, so cancelling at 98% orphaned the chunk and the next
attempt restarted from whatever the last COMPLETED attempt had left. On
an 840 MB model that meant re-fetching ~726 MB that was already on disk.
The chunk is now folded in on interruption too, which is sound because a
206 body starts exactly at `offset`, so partial + chunk is a valid longer
prefix (true even for a partly-written append, which copies
sequentially). The status is captured from `stateChange`, since an
interrupted transfer never resolves and a non-partial body must still be
discarded.
A cancelled transfer can also come back as a resolved task holding a
partial body, so the abort signal is re-checked before the response is
interpreted: navigating away mid-download is an abort, not a failure.
msluszniak
force-pushed
the
@ms/fetcher-verify-download-size
branch
from
August 17, 2026 12:42
491f5d1 to
0bee2ee
Compare
This was referenced Aug 18, 2026
barhanc
approved these changes
Aug 26, 2026
msluszniak
added a commit
that referenced
this pull request
Aug 26, 2026
Android downloads run on the system DownloadManager, so they continue while the app is backgrounded or killed. iOS streamed in-process through blob-util, which does not. Measured on device against a 314 MB file: suspending the app tore the connection down ONE SECOND later, 43 MB in, and the server logged the drop. For a 1-3 GB model that is the difference between a download finishing and never finishing, and it is one of the ways a partial file appeared in the first place. iOS only offers background transfers through a background NSURLSession, so this adds one, owned by the package rather than borrowed from blob-util. The awkward part is that a background session only does DOWNLOAD tasks, which stage into their own private file and hand it over whole at the end. There is no partially written file to append to, so the Range-based `.partial` resume from #1365 cannot be kept alongside it. Resume instead goes through NSURLSession's own resume data, which is persisted next to the destination, so it survives the app being killed and not merely suspended. Cancelling still keeps the bytes: `cancelByProducingResumeData:` rather than a plain `cancel`. Details that were not optional: * The session identifier is fixed rather than per-request, because that is what lets iOS hand a finished transfer back to a relaunched app. It also means the session outlives the JS: `startDownload` therefore adopts a task already running for the same destination instead of starting a second one, and `resetDownload` (which backs `forceDownload`) cancels it, or "download it again" would attach to the attempt it is meant to replace. * `discretionary` is set to NO. Background sessions otherwise let iOS defer a transfer on power or network grounds, which is wrong for a download the user just asked for. * `cancelByProducingResumeData:` returns its data asynchronously. Without waiting for it, a download restarted straight after an abort read the resume file before it was written and started from zero; observed exactly that, then fixed it by having a start wait on an in-flight cancel for the same file. * A background download task never fires `didReceiveResponse`, so there is no mid-transfer status any more. The completeness check from #1365 is unaffected: it compares the bytes on disk against the expected length. Verified on an iPhone 16, against a server logging what it actually sent: * Suspended at 12.8%, the transfer kept going for 129 s and 311 of 314 MB with no drop, then delivered completion and resolved. The control run, on the old path, dropped one second after suspension. * Aborted at p=0.0793 and downloaded again: resumed at p=0.0799, and the server saw `Range: bytes=25134704-` (exactly 0.0799 of the file) rather than a second full request. Part of #1366. Known limitation: a transfer that finishes while the app is KILLED needs the host app to forward `application:handleEventsForBackgroundURLSession:`. Without it, surviving suspension still works. Left as a follow-up.
msluszniak
added a commit
that referenced
this pull request
Aug 27, 2026
… the wire (#1368) ## Description Overall download progress did not track the bytes actually on the wire. It crawled, sat near zero for most of a large download, then jumped at the end. Measured on device against a 2.5 GiB body, the old bar reported `0.00 -> 0.20 -> 0.41 -> 0.62 -> -0.78 -> 1.00`; it now reports `0.00 -> 0.19 -> 0.39 -> 0.60 -> 0.81 -> 1.00`. Five causes, found by logging the raw native samples: 1. **Android lost the top bits of the byte counter past 2 GB.** blob-util reads DownloadManager's `COLUMN_BYTES_DOWNLOADED_SO_FAR` with `getInt` ([ReactNativeBlobUtilReq.java:213](https://github.com/RonRadtke/react-native-blob-util/blob/master/android/src/main/java/com/ReactNativeBlobUtil/ReactNativeBlobUtilReq.java#L213)), so JS receives the low 32 bits of a 64-bit counter as a signed int. On device the sample after `1668939776` was `-2057109504`: really 2,237,857,792 bytes, 83% of the file, reported as -77%. `Math.min(sum / total, 1)` had no lower bound to stop it. `COLUMN_TOTAL_SIZE_BYTES` uses `getLong`, so only the numerator was affected. The counter only grows, so the wraps are counted and the high bits restored. 2. **A single failed HEAD re-weighted every file.** Byte weighting applied only when every HEAD succeeded; one failure dropped all files to equal weighting, so a tokenizer counted as much as a 3 GB model. Weights are now per file, and an unmeasured file corrects itself once its transfer reports a real length. 3. **A finished file stayed short of its own size.** blob-util throttles progress events and DownloadManager is polled, so the final chunk lands between samples. A 711 KB file next to the 2.5 GiB one contributed zero for the whole run. Each file now reports its on-disk size once it lands. 4. **Cached files paid a HEAD they did not need.** The cache was consulted inside `downloadUrl`, after `download()` had already HEADed every URL for weighting. A fully cached config cost one round trip per file (2 of 2 on the warm run measured here, now 0). 5. **iOS sampled a third as often.** 20 callbacks per file against Android's 100, so a multi-GB download moved in 5% steps. Also, a response with no `Content-Length` reports `written: 0, total: -1` on a timer, dragging that file back to zero, and a resume started the bar at zero instead of at the bytes already on disk. The bar is also monotonic now: learning a weight mid-flight shifts the denominator, and a restarted transfer genuinely loses ground. ### Introduces a breaking change? - [ ] Yes - [x] No ### Type of change - [x] Bug fix (change which fixes an issue) - [ ] New feature (change which adds functionality) - [ ] Documentation update (improves or adds clarity to existing documentation) - [ ] Other (chores, tests, code style improvements etc.) ### Tested on - [x] iOS - [x] Android ### Testing instructions Progress only misbehaves visibly past 2 GB, so the file has to be larger than that. Downloading one over the internet takes too long to iterate on, so serve it locally: 1. Run a server that streams a >2 GB body with a correct `Content-Length` and `Accept-Ranges` (any static 2.5 GiB file works). 2. `adb reverse tcp:8099 tcp:8099`, so the device reaches it at `http://127.0.0.1:8099/big.bin` over the cable rather than over wifi. 3. Call `download('http://127.0.0.1:8099/big.bin', { forceDownload: true, onProgress: console.log })` and watch the values. Before this change the reported value goes negative shortly after 2 GB; after it, it rises monotonically. For point 4, add a second small URL, run once to populate the cache, then run again without `forceDownload` and count the HEAD requests reaching the server. ### Related issues Part of #1366. Stacked on #1365, which it uses the expected-length plumbing from. ### Checklist - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [x] My changes generate no new warnings ### Additional notes Point 5 is now verified on an iPhone 16 as well: a run whose `.partial` already held 8.5% of the file started the bar at `p=0.0852` instead of at zero, and produced 107 progress samples in smooth ~0.9% steps rather than the 20 the old count allowed. Point 1 is an upstream blob-util bug. Worth reporting there, but the workaround here is cheap and does not depend on a fix landing. The two remaining parts of #1366, iOS background downloads and large-file speed, follow in separate PRs.
msluszniak
added a commit
that referenced
this pull request
Aug 27, 2026
Android downloads run on the system DownloadManager, so they continue while the app is backgrounded or killed. iOS streamed in-process through blob-util, which does not. Measured on device against a 314 MB file: suspending the app tore the connection down ONE SECOND later, 43 MB in, and the server logged the drop. For a 1-3 GB model that is the difference between a download finishing and never finishing, and it is one of the ways a partial file appeared in the first place. iOS only offers background transfers through a background NSURLSession, so this adds one, owned by the package rather than borrowed from blob-util. The awkward part is that a background session only does DOWNLOAD tasks, which stage into their own private file and hand it over whole at the end. There is no partially written file to append to, so the Range-based `.partial` resume from #1365 cannot be kept alongside it. Resume instead goes through NSURLSession's own resume data, which is persisted next to the destination, so it survives the app being killed and not merely suspended. Cancelling still keeps the bytes: `cancelByProducingResumeData:` rather than a plain `cancel`. Details that were not optional: * The session identifier is fixed rather than per-request, because that is what lets iOS hand a finished transfer back to a relaunched app. It also means the session outlives the JS: `startDownload` therefore adopts a task already running for the same destination instead of starting a second one, and `resetDownload` (which backs `forceDownload`) cancels it, or "download it again" would attach to the attempt it is meant to replace. * `discretionary` is set to NO. Background sessions otherwise let iOS defer a transfer on power or network grounds, which is wrong for a download the user just asked for. * `cancelByProducingResumeData:` returns its data asynchronously. Without waiting for it, a download restarted straight after an abort read the resume file before it was written and started from zero; observed exactly that, then fixed it by having a start wait on an in-flight cancel for the same file. * A background download task never fires `didReceiveResponse`, so there is no mid-transfer status any more. The completeness check from #1365 is unaffected: it compares the bytes on disk against the expected length. Verified on an iPhone 16, against a server logging what it actually sent: * Suspended at 12.8%, the transfer kept going for 129 s and 311 of 314 MB with no drop, then delivered completion and resolved. The control run, on the old path, dropped one second after suspension. * Aborted at p=0.0793 and downloaded again: resumed at p=0.0799, and the server saw `Range: bytes=25134704-` (exactly 0.0799 of the file) rather than a second full request. Part of #1366. Known limitation: a transfer that finishes while the app is KILLED needs the host app to forward `application:handleEventsForBackgroundURLSession:`. Without it, surviving suspension still works. Left as a follow-up.
msluszniak
added a commit
that referenced
this pull request
Aug 27, 2026
Android downloads run on the system DownloadManager, so they continue while the app is backgrounded or killed. iOS streamed in-process through blob-util, which does not. Measured on device against a 314 MB file: suspending the app tore the connection down ONE SECOND later, 43 MB in, and the server logged the drop. For a 1-3 GB model that is the difference between a download finishing and never finishing, and it is one of the ways a partial file appeared in the first place. iOS only offers background transfers through a background NSURLSession, so this adds one, owned by the package rather than borrowed from blob-util. The awkward part is that a background session only does DOWNLOAD tasks, which stage into their own private file and hand it over whole at the end. There is no partially written file to append to, so the Range-based `.partial` resume from #1365 cannot be kept alongside it. Resume instead goes through NSURLSession's own resume data, which is persisted next to the destination, so it survives the app being killed and not merely suspended. Cancelling still keeps the bytes: `cancelByProducingResumeData:` rather than a plain `cancel`. Details that were not optional: * The session identifier is fixed rather than per-request, because that is what lets iOS hand a finished transfer back to a relaunched app. It also means the session outlives the JS: `startDownload` therefore adopts a task already running for the same destination instead of starting a second one, and `resetDownload` (which backs `forceDownload`) cancels it, or "download it again" would attach to the attempt it is meant to replace. * `discretionary` is set to NO. Background sessions otherwise let iOS defer a transfer on power or network grounds, which is wrong for a download the user just asked for. * `cancelByProducingResumeData:` returns its data asynchronously. Without waiting for it, a download restarted straight after an abort read the resume file before it was written and started from zero; observed exactly that, then fixed it by having a start wait on an in-flight cancel for the same file. * A background download task never fires `didReceiveResponse`, so there is no mid-transfer status any more. The completeness check from #1365 is unaffected: it compares the bytes on disk against the expected length. Verified on an iPhone 16, against a server logging what it actually sent: * Suspended at 12.8%, the transfer kept going for 129 s and 311 of 314 MB with no drop, then delivered completion and resolved. The control run, on the old path, dropped one second after suspension. * Aborted at p=0.0793 and downloaded again: resumed at p=0.0799, and the server saw `Range: bytes=25134704-` (exactly 0.0799 of the file) rather than a second full request. Part of #1366. Known limitation: a transfer that finishes while the app is KILLED needs the host app to forward `application:handleEventsForBackgroundURLSession:`. Without it, surviving suspension still works. Left as a follow-up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes the way how models are redownloaded after interruption by switching them in the middle of downloading etc. Previously, it caused errors, now it clearly re-start download from the point where it stopped.
Introduces a breaking change?
Type of change
Tested on
Testing instructions
Download of ... is incomplete (N of M bytes)rather than caching a short file, and retrying resumes.Verified on device across the CV app's models. Android is unverified: DownloadManager exposes no status and cancel-then-resume goes through a different code path, so it needs its own pass before merge.
Related issues
Follow-ups split out into #1366.
Checklist
Additional notes